-
Notifications
You must be signed in to change notification settings - Fork 1
[FEAT] #197: 신규 멤버 모집 관련 관리자 페이지 api 구현 #204
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Caution Review failedThe pull request is closed. Walkthrough리크루팅 지원자 결제 상태 PATCH 엔드포인트와 이름 검색/페이징 지원 GET 목록 엔드포인트가 추가되었고, 입학 학기 관련 enum·유틸·엔티티 필드·마이그레이션이 도입되어 기존 데이터 백필 및 NOT NULL 제약과 인덱스가 적용되었다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Admin as Client
participant C as RecruitMemberController
participant S as RecruitMemberService
participant R as RecruitMemberRepository
participant E as RecruitMember
rect rgba(200,230,255,0.25)
note over Admin,C: PATCH /api/v1/recruit/members/{id}/payment
Admin->>C: PaymentUpdateRequest{isPayed}
C->>S: updatePayment(memberId, isPayed)
S->>R: findById(memberId)
R-->>S: RecruitMember or null
alt Member exists
alt isPayed != current
S->>E: markPaid()/markUnpaid()
else Already desired state
note over S: No-op (idempotent)
end
S-->>C: void
C-->>Admin: ApiResponse(message=결과 메시지)
else Not found
S-->>C: throws RecruitMemberException
C-->>Admin: Error response
end
end
sequenceDiagram
autonumber
actor Admin as Client
participant C as RecruitMemberController
participant S as RecruitMemberService
participant R as RecruitMemberRepository
participant P as Page<RecruitMember>
rect rgba(220,255,220,0.25)
note over Admin,C: GET /api/v1/recruit/members?question=&page=&size=&sort=&dir=
Admin->>C: query params
C->>C: Pageable 생성(page/size/sort/dir)
alt question present
C->>S: searchMembersByNamePage(question, pageable)
S->>R: findByNameContainingIgnoreCase(question, pageable)
else no question
C->>S: findAllMembersPage(pageable)
S->>R: findAll(pageable)
end
R-->>S: Page<RecruitMember>
S-->>C: Page<RecruitMember>
C->>C: map to List<RecruitMemberSummaryResponse>\nPageMeta.of(Page)
C-->>Admin: ApiResponse(data=list, meta=PageMeta, message=목록 조회 성공)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
CI status |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (20)
src/main/java/inha/gdgoc/domain/recruit/dto/request/PaymentUpdateRequest.java (1)
3-5: 네이밍 정합성: isPayed → isPaid/paid 권장 (영문 철자 개선, API 가독성 향상)도메인 전반에
isPayed가 이미 사용 중인 점은 이해하지만, 공개 API 필드로 계속 노출하면 장기적으로 혼란을 유발합니다. 점진적 마이그레이션을 제안합니다: 요청 DTO는paid를 노출하고@JsonAlias("isPayed")로 하위 호환 유지, 응답 DTO에도 동일 패턴 적용.원하시면 전체 레포에서
isPayed참조를 수집하고 변경 영향도를 리포트하는 스크립트를 제공하겠습니다.src/main/java/inha/gdgoc/domain/recruit/enums/AdmissionSemester.java (1)
3-5: 열거형 고정 대신 값 객체/문자열 저장 고려매 해마다 enum 상수를 추가해야 하는 운영 부담이 큽니다. DB는
VARCHAR로 저장 중이므로, 자바 측도 enum 대신 Year-Semester 값 객체(예:record YearSemester(int year, int term)) 또는 검증된 문자열을 사용하는 방안도 검토해 보세요.src/main/java/inha/gdgoc/global/util/SemesterCalculator.java (3)
13-16: 열거 미존재 시 예외 타입 및 메시지 개선, 운영 가드 필요현재는 모든 예외를 묶어
RuntimeException으로 재던집니다. 상태 불일치에 가까우므로IllegalStateException이 더 적절합니다. 또한 “다음 해 상수 추가 필요”를 명확히 표기하고, 로그/모니터링에서 탐지 가능하도록 에러 코드를 포함하세요.- try { - return AdmissionSemester.valueOf(enumName); - } catch (Exception ex) { - throw new RuntimeException( - "AdmissionSemester enum에 상수 " + enumName + " 이(가) 정의되어 있지 않습니다. " + - "해당 연도/학기 상수를 추가하세요.", ex - ); - } + try { + return AdmissionSemester.valueOf(enumName); + } catch (IllegalArgumentException ex) { + throw new IllegalStateException( + "[SEMESTER_ENUM_MISSING] AdmissionSemester에 " + enumName + + " 상수가 없습니다. 다음 해/학기 상수를 선반영하세요.", ex + ); + }Also applies to: 35-43
24-33: 2월(02) 처리 도메인 검증 필요: 현재 로직은 2월을 ‘해당 연도 1학기’로 산정국내 학사 캘린더 기준으로 2월은 방학(전년도 2학기)에 귀속시키는 경우가 많습니다. 의도된 비즈니스 규칙인지 확인이 필요합니다. 규칙이 다르면 분기 조건을 조정하고 경계 월(1/2/3/8)의 회귀 테스트를 추가하세요.
원하시면 경계 날짜(01-31, 02-01, 03-01, 07-31, 08-01, 12-31)에 대한 파라미터라이즈드 테스트 템플릿을 제공하겠습니다.
21-23: 연산 안전성(가독성) 개선 여지
yy = (year - 1) % 100대신Math.floorMod(year - 1, 100)사용을 고려하면 음수 연도 입력 등 엣지 케이스에서 더 안전합니다(의미상 큰 차이는 없으나 명확성 향상).src/main/java/inha/gdgoc/domain/recruit/dto/request/RecruitMemberRequest.java (2)
29-29: 요청 DTO에 isPayed 노출은 혼란 유발: 생성 시 항상 false로 고정됨
toEntity()에서.isPayed(false)로 강제되므로 클라이언트가 값을 보내도 무시됩니다. 생성 API에서 결제 상태를 받지 않는다면, 필드를 제거하거나 역직렬화 불가로 처리해 혼동을 줄이세요.두 가지 대안:
- 아예 필드 제거
- 역직렬화 차단
+import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.annotation.JsonProperty.Access; ... - private Boolean isPayed; + @JsonProperty(access = Access.READ_ONLY) + private Boolean isPayed;
45-46: 현재 학기 자동 지정은 합리적이나, enum 범위 누락 시 런타임 예외 전파 가능
SemesterCalculator.currentSemester()가 enum 미존재로 실패하면 신규 지원 생성 자체가 막힙니다. 상수 선반영(AdmissionSemester.java 코멘트 참조) 또는 컨트롤러/서비스에서 예외 메시지를 사용자 친화적으로 매핑하세요.src/main/java/inha/gdgoc/domain/recruit/repository/RecruitMemberRepository.java (1)
11-11: 이름 부분검색 페이징 메서드 추가: 사용성 측면에서 LGTM메서드 시그니처와 반환 타입(Page)이 적절합니다. 다만 대용량 데이터에서
ContainingIgnoreCase는 함수적 인덱스를 타지 못할 수 있어(LOWER(name) 사용) 성능 이슈가 생길 수 있습니다. DB에 따라 함수 인덱스/CI(T)EXT/대소문자 무시 컬레이션 등을 검토하세요.성능 개선용 인덱스 예시(PostgreSQL):
CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_recruit_member_name_lower ON recruit_member (LOWER(name));src/main/resources/db/migration/V20250826__admission_semester_add_backfill_enforce.sql (3)
6-15: 백필 로직의 경계월/타임존/NULL 위험 — NOT NULL 전환 시 장애 가능성
- 1월을 전년도 2학기로 매핑하는 규칙과 2
7/812 경계가 비즈니스 규칙과 정확히 일치하는지 확인 필요.- created_at 컬럼이 UTC라면 학기 경계(특히 1월/8월 초)에서 오분류 가능성 있음. KST 기준이 요구된다면 EXTRACT 전에 AT TIME ZONE 'Asia/Seoul' 고려.
- created_at가 NULL인 레코드가 존재하면 UPDATE 후에도 admission_semester가 NULL로 남아 SET NOT NULL 단계에서 실패합니다.
사전 점검 쿼리 제안:
- SELECT COUNT(*) FROM recruit_member WHERE created_at IS NULL;
- SELECT EXTRACT(MONTH FROM created_at) AS m, COUNT(*) FROM recruit_member GROUP BY m ORDER BY m;
주석 가독성도 개선해두면 좋습니다. 예시 diff:
- -- 학기 규칙: 2~7월 = YYY_1, 8~12월 = YYY_2, 1월 = (전년도) YYY_2 + -- 학기 규칙(KST 가정): 2~7월 = YY_1, 8~12월 = YY_2, 1월 = (전년도) YY_2 + -- 예시: 2025-03 ⇒ Y25_1, 2025-10 ⇒ Y25_2, 2025-01 ⇒ Y24_2
19-20: NOT NULL 전환 전 사전 검증/보정 절차 필요SET NOT NULL는 남아있는 NULL을 즉시 실패시킵니다. 마이그레이션 안정성을 위해:
- (권장) 백필 후 잔존 NULL 개수 점검 및 실패 시 롤백/중단.
- (대안) created_at NULL 레코드에 대한 보정 규칙 명시(예: 현재 학기 적용 또는 수동 처리 리스트 작성).
NULL 잔존 점검 쿼리:
- SELECT COUNT(*) FROM recruit_member WHERE admission_semester IS NULL;
23-24: 대용량 운영 테이블 고려 시 인덱스 락 완화 권고운영 데이터가 많다면 CREATE INDEX가 테이블에 공유락을 유발할 수 있습니다. PostgreSQL이라면 CONCURRENTLY 옵션을 고려하세요. 단, Flyway SQL 마이그레이션은 트랜잭션 안에서 실행되므로 CONCURRENTLY 사용 시 해당 마이그레이션을 non-transactional로 분리해야 합니다.
적용 예시(별도 non-Tx 마이그레이션으로 분리 권장):
-CREATE INDEX idx_recruit_member_admission_semester - ON recruit_member (admission_semester); +-- 대용량 환경에서는 다음과 같이 CONCURRENTLY 사용을 검토하세요 (트랜잭션 밖에서 실행 필요). +CREATE INDEX CONCURRENTLY idx_recruit_member_admission_semester + ON recruit_member (admission_semester);src/main/java/inha/gdgoc/domain/recruit/controller/message/RecruitMemberMessage.java (1)
8-10: 용어 일관성(입금/결제, 완료/미완료) 점검 제안도메인 전반에서 payment/deposit, paid/unpaid 표현 혼재 가능성이 있습니다. 메시지/필드명/엔드포인트 응답 문구의 용어를 한 번에 정리하면 유지보수성이 좋아집니다. 예: PAYMENT_STATUS_UPDATE_SUCCESS처럼 상태 업데이트 메시지로 일반화.
src/main/java/inha/gdgoc/domain/recruit/entity/RecruitMember.java (1)
74-76: isPayed 명명 관련 사소한 컨벤션 제안영문 관례상 paid가 맞춤법에 부합합니다. 도메인 전반에서 isPayed가 널리 사용 중이면 유지가 합리적이지만, 향후 마이그레이션 여지가 있다면 isPaid로 정리하는 것을 권장합니다.
src/main/java/inha/gdgoc/global/dto/response/PageMeta.java (1)
18-37: 정렬 메타 계산 로직 개선: 불필요한 reduce 제거 및 UNSORTED 정확히 표현
- 여러 정렬 키를 연결할 때는 reduce 대신 Collectors.joining 사용이 간단하고 안전합니다.
- 정렬이 지정되지 않은 경우에도 현재 코드는 "createdAt/DESC"를 임의 기본값으로 노출합니다. 실제 정렬이 UNSORTED라면 null(or 빈 문자열)로 표현하는 편이 정확합니다.
적용 예시 diff:
+import java.util.stream.Collectors; @@ - String sortProps = page.getSort().stream() - .map(Sort.Order::getProperty) - .reduce((a, b) -> a + "," + b) - .orElse("createdAt"); + String sortProps = page.getSort().isSorted() + ? page.getSort().stream() + .map(Sort.Order::getProperty) + .collect(Collectors.joining(",")) + : null; @@ - String dir = page.getSort().stream() - .findFirst() - .map(o -> o.getDirection().name()) - .orElse("DESC"); + String dir = page.getSort().isSorted() + ? page.getSort().stream() + .findFirst() + .map(o -> o.getDirection().name()) + .orElse(null) + : null;src/main/java/inha/gdgoc/domain/recruit/dto/response/RecruitMemberSummaryResponse.java (2)
16-21: 학기 표시 문자열 포맷 책임의 위치 이동 권고현재 포맷("Y25_1" → "25-1")을 DTO에서 substring/replace로 직접 가공하고 있습니다. 이는 enum/값객체의 표현 규칙이 바뀔 때 DTO마다 수정이 필요해 응집도가 떨어집니다. AdmissionSemester(또는 값객체)에 displayCode()/toDisplay()를 두고 DTO에서는 그 메서드를 호출하는 구조가 바람직합니다.
(전제: enum 유지 시) 사용 예시:
- String enumName = recruitMember.getAdmissionSemester().name(); - semester = enumName.substring(1).replace('_', '-'); + semester = recruitMember.getAdmissionSemester().toDisplay(); // 예: "25-1"AdmissionSemester에 추가(참고용):
public enum AdmissionSemester { Y25_1, Y25_2, Y26_1, Y26_2; // … 확장 또는 값객체 전환 public String toDisplay() { String n = name(); // "Y25_1" return n.substring(1).replace('_','-'); // "25-1" } }
22-30: Boolean null 안전성 재확인(사소)엔티티에서 isPayed는 nullable=false이므로 현재 매핑은 안전합니다. 다만 과거 데이터 이행 도중 null 가능성이 있었다면 Boolean.TRUE.equals(...) 패턴(다른 DTO에 이미 사용됨)을 통일적으로 쓰는 것도 방법입니다.
src/main/java/inha/gdgoc/domain/recruit/service/RecruitMemberService.java (3)
75-77: 엔티티 미존재 시 도메인 예외로 통일 권장동일 클래스의
findSpecifiedMember는RecruitMemberException(RECRUIT_MEMBER_NOT_FOUND)를 사용합니다. 여기서만IllegalArgumentException을 던지면 전역 예외 처리 및 응답 포맷이 불일치할 수 있습니다. 도메인 예외로 통일하는 것을 권장합니다.적용 예시(diff):
- RecruitMember m = recruitMemberRepository.findById(memberId) - .orElseThrow(() -> new IllegalArgumentException("RecruitMember not found: " + memberId)); + RecruitMember m = recruitMemberRepository.findById(memberId) + .orElseThrow(() -> new RecruitMemberException(RECRUIT_MEMBER_NOT_FOUND));
74-74: 이름 표기 isPayed vs isPaid도메인 전반에서
isPayed를 사용하고 있는데, 일반적으로는paid철자를 씁니다. 지금 PR에서의 리네이밍은 영향도가 크니 보류하되, 추후 마이그레이션 계획(필드/칼럼명 동시 변경)을 고려해 보세요.팀 컨벤션에 따라 그대로 유지할지, 다음 릴리스에서 리네이밍할지 결정이 필요합니다.
85-93: 페이지 조회 메서드 구성은 적절합니다 + 클래스 레벨 readOnly 기본값 제안읽기 전용 트랜잭션으로 잘 선언되었습니다. 서비스 클래스 레벨에
@Transactional(readOnly = true)를 두고, 쓰기 메서드(addRecruitMember,updatePayment)만@Transactional로 override하면 가독성과 안전성이 올라갑니다.StudyAttendeeService와도 스타일이 일치합니다.src/main/java/inha/gdgoc/domain/recruit/controller/RecruitMemberController.java (1)
98-118: 입금 상태 변경 API: 요청 유효성 검증(@Valid) 추가 및 no-op 응답 명확화 제안
- DTO 제약이 있어도
@Valid가 없으면 검증이 트리거되지 않습니다.@Valid추가를 권장합니다.- 현재는 요청 상태와 동일한 상태여도(서비스의 멱등 스킵) “변경했습니다” 메시지를 반환합니다. 실제 변경 여부를 반영하거나, 멱등 스킵 시 별도 메시지를 주는 방안 검토를 권장합니다.
적용 예시(diff):
- @RequestBody PaymentUpdateRequest paymentUpdateRequest + @Valid @RequestBody PaymentUpdateRequest paymentUpdateRequest추가 import:
// 추가 import 필요 import jakarta.validation.Valid;옵션:
- 응답 본문에 최종 상태를 포함시키면(예:
{ "payed": true }) 클라이언트가 멱등/실변경을 쉽게 구분할 수 있습니다.- 서비스에서 도메인 예외를 일관되게 던지도록 정리하면(현재
IllegalArgumentException혼재) 에러 응답 표준화에 도움이 됩니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (12)
src/main/java/inha/gdgoc/domain/recruit/controller/RecruitMemberController.java(3 hunks)src/main/java/inha/gdgoc/domain/recruit/controller/message/RecruitMemberMessage.java(1 hunks)src/main/java/inha/gdgoc/domain/recruit/dto/request/PaymentUpdateRequest.java(1 hunks)src/main/java/inha/gdgoc/domain/recruit/dto/request/RecruitMemberRequest.java(2 hunks)src/main/java/inha/gdgoc/domain/recruit/dto/response/RecruitMemberSummaryResponse.java(1 hunks)src/main/java/inha/gdgoc/domain/recruit/entity/RecruitMember.java(2 hunks)src/main/java/inha/gdgoc/domain/recruit/enums/AdmissionSemester.java(1 hunks)src/main/java/inha/gdgoc/domain/recruit/repository/RecruitMemberRepository.java(1 hunks)src/main/java/inha/gdgoc/domain/recruit/service/RecruitMemberService.java(2 hunks)src/main/java/inha/gdgoc/global/dto/response/PageMeta.java(1 hunks)src/main/java/inha/gdgoc/global/util/SemesterCalculator.java(1 hunks)src/main/resources/db/migration/V20250826__admission_semester_add_backfill_enforce.sql(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (8)
src/main/java/inha/gdgoc/domain/recruit/dto/request/PaymentUpdateRequest.java (1)
src/main/java/inha/gdgoc/domain/recruit/dto/response/SpecifiedMemberResponse.java (1)
SpecifiedMemberResponse(8-29)
src/main/java/inha/gdgoc/domain/recruit/controller/message/RecruitMemberMessage.java (6)
src/main/java/inha/gdgoc/domain/game/controller/message/GameUserMessage.java (1)
GameUserMessage(3-6)src/main/java/inha/gdgoc/domain/auth/controller/message/AuthMessage.java (1)
AuthMessage(3-11)src/main/java/inha/gdgoc/domain/study/controller/message/StudyMessage.java (1)
StudyMessage(3-9)src/main/java/inha/gdgoc/domain/recruit/exception/RecruitMemberErrorCode.java (2)
Override(21-24)RequiredArgsConstructor(7-25)src/main/java/inha/gdgoc/domain/study/controller/message/StudyAttendeeMessage.java (1)
StudyAttendeeMessage(3-8)src/main/java/inha/gdgoc/domain/resource/controller/message/ResourceMessage.java (1)
ResourceMessage(3-5)
src/main/java/inha/gdgoc/global/dto/response/PageMeta.java (2)
src/main/java/inha/gdgoc/global/dto/response/ErrorMeta.java (1)
ErrorMeta(3-8)src/main/java/inha/gdgoc/global/dto/response/ApiResponse.java (1)
JsonInclude(6-36)
src/main/java/inha/gdgoc/domain/recruit/entity/RecruitMember.java (3)
src/main/java/inha/gdgoc/domain/recruit/dto/request/RecruitMemberRequest.java (1)
Builder(13-48)src/main/java/inha/gdgoc/domain/recruit/entity/Answer.java (1)
Entity(24-58)src/main/java/inha/gdgoc/domain/recruit/dto/response/SpecifiedMemberResponse.java (1)
SpecifiedMemberResponse(8-29)
src/main/java/inha/gdgoc/domain/recruit/dto/response/RecruitMemberSummaryResponse.java (1)
src/main/java/inha/gdgoc/domain/recruit/dto/response/SpecifiedMemberResponse.java (2)
SpecifiedMemberResponse(8-29)from(16-28)
src/main/java/inha/gdgoc/domain/recruit/service/RecruitMemberService.java (1)
src/main/java/inha/gdgoc/domain/study/service/StudyAttendeeService.java (2)
Transactional(148-172)Slf4j(40-189)
src/main/java/inha/gdgoc/domain/recruit/controller/RecruitMemberController.java (2)
src/main/java/inha/gdgoc/domain/recruit/controller/message/RecruitMemberMessage.java (1)
RecruitMemberMessage(3-11)src/main/java/inha/gdgoc/domain/recruit/service/RecruitMemberService.java (1)
RequiredArgsConstructor(24-95)
src/main/java/inha/gdgoc/domain/recruit/dto/request/RecruitMemberRequest.java (1)
src/main/java/inha/gdgoc/global/util/SemesterCalculator.java (1)
SemesterCalculator(8-45)
🔇 Additional comments (8)
src/main/java/inha/gdgoc/domain/recruit/controller/message/RecruitMemberMessage.java (1)
8-10: 메시지 상수 추가사항 문제 없음컨벤션(대문자 스네이크, “…성공…” 톤)과 다른 Message 클래스들과의 톤/서술이 일관적입니다. 컨트롤러에서의 사용도 명확합니다.
src/main/java/inha/gdgoc/domain/recruit/entity/RecruitMember.java (2)
85-91: 결제 상태 토글 도메인 메서드 추가 적절isPayed null 안전(Boolean.TRUE/FALSE 지정)하며, 애그리게잇 내부에서 의미 있는 행위를 노출하는 좋은 캡슐화입니다.
4-4: SemesterCalculator 로직 및 시간대(KST) 백필 스크립트 일치 여부 확인 필요현재
src/main/java/inha/gdgoc/global/util/SemesterCalculator.java의currentSemester()메서드는
ZoneId.of("Asia/Seoul")기반 KST 시간대를 사용of(LocalDate date)구현은
month == 1→ 이전 연도 2학기2 ≤ month ≤ 7→ 해당 연도 1학기8 ≤ month ≤ 12→ 해당 연도 2학기위 로직은 리뷰에서 제안한 마이그레이션 규칙(2
7월=1학기, 812월=2학기, 1월=전년도 2학기)에 부합합니다.다만, 레거시 데이터 백필/마이그레이션 스크립트(
.sql또는 Java 기반 Migration)에서 동일한 로직과 KST 시간대를 사용하고 있는지 반드시 검증해야 합니다. 불일치할 경우 신규 데이터(RecruitMemberRequest)와 과거 백필 데이터 간admissionSemester값이 혼재될 수 있습니다.검증 항목:
- 백필/마이그레이션 스크립트가
SemesterCalculator.of()또는currentSemester()를 호출하도록 구현되었는지- 스크립트 실행 시 JVM 기본 타임존이 반드시
Asia/Seoul로 설정되어 있는지이상 없으면 본 코멘트는 해결 처리하셔도 됩니다.
src/main/java/inha/gdgoc/domain/recruit/service/RecruitMemberService.java (2)
19-23: Spring @transactional로 일원화한 점 좋습니다패키지
org.springframework.transaction.annotation.Transactional사용으로 일관성이 생겼고 테스트/프록시 동작도 예측 가능합니다.
78-83: 이슈 없음:isPayed필드는 절대 null이 될 수 없습니다
RecruitMember엔티티의isPayed필드는@Column(nullable = false)로 선언되어 있어, DB에 null 값이 저장될 수 없습니다.RecruitMemberRequest.toEntity()에서도 항상.isPayed(false)로 초기값을 설정하기 때문에, 애플리케이션 레벨에서도 null 상태가 발생하지 않습니다.- 따라서 현재의 멱등 체크 로직
은if (Boolean.TRUE.equals(m.getIsPayed()) == isPayed) return;(현재 유료 여부) == (요청 유료 여부)조건만으로 올바르게 idempotence를 보장하며, null 분기를 추가할 필요가 없습니다.👉 제안된 null 처리 로직은 실제 사용 시나리오에서 불필요하므로, 원본 코드를 그대로 유지해 주세요.
Likely an incorrect or invalid review comment.
src/main/java/inha/gdgoc/domain/recruit/controller/RecruitMemberController.java (3)
3-7: 메시지 상수 분리와 사용 일관성 좋습니다응답 메시지 상수를 재사용해 유지보수성이 좋아졌습니다.
45-45: Swagger 태그 추가 좋습니다API 그룹이 명확해져 문서 탐색성이 향상됩니다.
150-156: 페이지네이션 변환 파이프라인 깔끔합니다
Page#map으로 DTO 변환 후PageMeta를 별도로 구성하는 방식이 명확합니다.
| @Operation( | ||
| summary = "지원자 목록 조회", | ||
| description = "전체 목록 또는 이름 검색 결과를 반환합니다. 검색어(question)를 주면 이름 포함 검색, 없으면 전체 조회. sort랑 dir은 example 값 그대로 코딩하는 것 추천...", | ||
| security = { @SecurityRequirement(name = "BearerAuth") } | ||
| ) | ||
| @PreAuthorize("hasRole('ADMIN')") | ||
| @GetMapping("/recruit/members") | ||
| public ResponseEntity<ApiResponse<List<RecruitMemberSummaryResponse>, PageMeta>> getMembers( | ||
| @Parameter(description = "검색어(이름 부분 일치). 없으면 전체 조회", example = "소연") | ||
| @RequestParam(required = false) String question, | ||
|
|
||
| @Parameter(description = "페이지(0부터 시작)", example = "0") | ||
| @RequestParam(defaultValue = "0") int page, | ||
|
|
||
| // TODO 입금 완료 | ||
| @Parameter(description = "페이지 크기", example = "20") | ||
| @RequestParam(defaultValue = "20") int size, | ||
|
|
||
| @Parameter(description = "정렬 필드", example = "createdAt") | ||
| @RequestParam(defaultValue = "createdAt") String sort, | ||
|
|
||
| @Parameter(description = "정렬 방향 ASC/DESC", example = "DESC") | ||
| @RequestParam(defaultValue = "DESC") String dir | ||
| ) { | ||
| Direction direction = "ASC".equalsIgnoreCase(dir) ? Direction.ASC : Direction.DESC; | ||
| Pageable pageable = PageRequest.of(page, size, Sort.by(direction, sort)); | ||
|
|
||
| Page<RecruitMember> memberPage = (question == null || question.isBlank()) | ||
| ? recruitMemberService.findAllMembersPage(pageable) | ||
| : recruitMemberService.searchMembersByNamePage(question, pageable); | ||
|
|
||
| List<RecruitMemberSummaryResponse> list = memberPage | ||
| .map(RecruitMemberSummaryResponse::from) | ||
| .getContent(); | ||
| PageMeta meta = PageMeta.of(memberPage); | ||
|
|
||
| return ResponseEntity.ok(ApiResponse.ok(MEMBER_LIST_RETRIEVED_SUCCESS, list, meta)); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
목록 조회 API: 파라미터 검증 추가와 정렬 컬럼 화이트리스트 권장
- page/size 음수 등으로
PageRequest.of가 예외를 던지면 500으로 번질 수 있습니다. Bean Validation으로 400을 반환하도록 하는 것이 안전합니다. - 임의의
sort값은 스프링 데이터에서PropertyReferenceException을 유발할 수 있습니다. 허용 컬럼을 제한하세요.
검증 추가(diff):
- @RequestParam(defaultValue = "0") int page,
+ @Min(0) @RequestParam(defaultValue = "0") int page,
@@
- @RequestParam(defaultValue = "20") int size,
+ @Min(1) @Max(100) @RequestParam(defaultValue = "20") int size,정렬 컬럼 화이트리스트(예시)(diff):
- Direction direction = "ASC".equalsIgnoreCase(dir) ? Direction.ASC : Direction.DESC;
- Pageable pageable = PageRequest.of(page, size, Sort.by(direction, sort));
+ Direction direction = "ASC".equalsIgnoreCase(dir) ? Direction.ASC : Direction.DESC;
+ // 실제 엔티티 필드에 맞게 컬럼 목록을 조정하세요.
+ String sortKey = switch (sort) {
+ case "createdAt", "name", "admissionSemester" -> sort;
+ default -> "createdAt";
+ };
+ Pageable pageable = PageRequest.of(page, size, Sort.by(direction, sortKey));추가 import:
// 추가 import 필요
import jakarta.validation.constraints.Min;
import jakarta.validation.constraints.Max;운영 수위 제안:
- size 상한은 운영 부하를 고려해 50~100선에서 조정하세요(현재 예시로 100 사용).
- 필요 시
question = question == null ? null : question.trim();으로 공백만 있는 입력을 일찍 정규화할 수 있습니다.
| public record PaymentUpdateRequest( | ||
| boolean isPayed | ||
| ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
PATCH DTO에서 primitive boolean 사용은 위험합니다: 누락 시 false로 바인딩되어 오작동 가능
PATCH 시 필드가 누락되면 primitive boolean은 false로 디시리얼라이즈되어 의도치 않게 “미납”으로 변경될 수 있습니다. Boolean + Bean Validation(@NotNull)로 바꾸고, 컨트롤러 파라미터에 @Valid를 적용해 주세요.
권장 수정안:
+import jakarta.validation.constraints.NotNull;
public record PaymentUpdateRequest(
- boolean isPayed
+ @NotNull Boolean isPayed
) {
}컨트롤러에서의 유효성 검사 적용도 확인 필요합니다:
🏁 Script executed:
#!/bin/bash
# PaymentUpdateRequest가 @Valid로 검증되는지 확인
rg -nP -C3 '@PatchMapping\([^)]*payment[^)]*\)' src/main/java | sed -n '1,120p'
rg -n 'PaymentUpdateRequest' src/main/java | sed -n '1,120p'Length of output: 1487
PATCH DTO와 컨트롤러 유효성 검사 개선 필요
원격 PATCH 요청에서 primitive boolean을 그대로 쓰면, 클라이언트가 해당 필드를 아예 보내지 않았을 때(false로 디시리얼라이즈) 의도치 않은 “미납” 상태로 변경될 위험이 있습니다. 또한 컨트롤러 메서드에 @Valid 검증이 빠져 있어, Bean Validation이 전혀 동작하지 않습니다.
수정해야 할 지점:
- src/main/java/inha/gdgoc/domain/recruit/dto/request/PaymentUpdateRequest.java
• 필드 타입을 primitiveboolean→ 객체형Boolean으로 변경
•@NotNull애너테이션 추가 → 누락 시 400 Bad Request 반환 - src/main/java/inha/gdgoc/domain/recruit/controller/RecruitMemberController.java
•@RequestBody PaymentUpdateRequest paymentUpdateRequest→@Valid @RequestBody PaymentUpdateRequest paymentUpdateRequest로 변경
•import jakarta.validation.Valid;(또는javax.validation.Valid;) 추가
권장 수정 예시:
--- a/src/main/java/inha/gdgoc/domain/recruit/dto/request/PaymentUpdateRequest.java
+++ b/src/main/java/inha/gdgoc/domain/recruit/dto/request/PaymentUpdateRequest.java
@@
-public record PaymentUpdateRequest(
- boolean isPayed
-) {
+import jakarta.validation.constraints.NotNull;
+
+public record PaymentUpdateRequest(
+ @NotNull Boolean isPayed
+) {
}--- a/src/main/java/inha/gdgoc/domain/recruit/controller/RecruitMemberController.java
+++ b/src/main/java/inha/gdgoc/domain/recruit/controller/RecruitMemberController.java
@@
- public ResponseEntity<ApiResponse<Void, Void>> updatePayment(
- @PathVariable Long memberId,
- @RequestBody PaymentUpdateRequest paymentUpdateRequest
+) public ResponseEntity<ApiResponse<Void, Void>> updatePayment(
+ @PathVariable Long memberId,
+ @Valid @RequestBody PaymentUpdateRequest paymentUpdateRequest위 두 군데를 반영한 뒤에, 실제로 유효성 검사 예외가 컨트롤러로 전달되는지 통합테스트 등을 통해 반드시 확인해 주세요.
🤖 Prompt for AI Agents
In src/main/java/inha/gdgoc/domain/recruit/dto/request/PaymentUpdateRequest.java
(around lines 3-5) change the primitive field type from boolean to the wrapper
Boolean and annotate it with @NotNull so missing JSON field yields a 400; add
the necessary import for jakarta.validation.constraints.NotNull. In
src/main/java/inha/gdgoc/domain/recruit/controller/RecruitMemberController.java
update the controller method signature to accept @Valid @RequestBody
PaymentUpdateRequest paymentUpdateRequest and add the import for
jakarta.validation.Valid so Bean Validation runs and missing/invalid payloads
are rejected.
| @Enumerated(EnumType.STRING) | ||
| @Column(name = "admission_semester", nullable = false, length = 10) | ||
| private AdmissionSemester admissionSemester; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Enum 매핑과 마이그레이션 값 불일치 가능 — 과거 데이터 로딩 실패 위험(중대)
DB 마이그레이션은 과거 레코드에 Y24_1, Y24_2 … 형태를 백필합니다. 그런데 AdmissionSemester enum이 Y25_1, Y25_2, Y26_1, Y26_2 정도로 제한되어 있다면, JPA @Enumerated(EnumType.STRING) 매핑은 과거 값 로딩 시 즉시 IllegalArgumentException(No enum constant …)로 실패합니다. 목록 조회/검색 시 런타임 장애로 이어집니다.
해결 옵션:
- 옵션 A(가장 유연): JPA 필드를 String으로 보관하고, 패턴 검증(DB CHECK + Bean Validation)과 값 객체(파서/포매터)로 도메인 계층에서 타입안전 확보.
- 옵션 B(차선): AttributeConverter로 문자열(Y25_1 등)을 파싱해 커스텀 값 객체(비 enum)로 매핑.
- 옵션 C(단기): Enum을 과거
미래 범위까지 대량 확장(운영 히스토리에 맞춰 최소 20202035 등). 다만 유지보수 비용 큽니다.
필드만 String으로 전환하는 최소 수정안 예시:
- @Enumerated(EnumType.STRING)
- @Column(name = "admission_semester", nullable = false, length = 10)
- private AdmissionSemester admissionSemester;
+ // 과거/미래 학기 값을 폭넓게 수용하기 위해 문자열로 저장합니다.
+ @Column(name = "admission_semester", nullable = false, length = 10)
+ private String admissionSemester;후속 영향:
- RecruitMemberRequest.toEntity()가 SemesterCalculator.currentSemester() (enum 반환) 사용 중이면, 문자열 코드(예: "Y25_1")를 반환하는 헬퍼로 교체 필요.
- Summary/Specified DTO 매핑에서 enum.name() 접근 부를 문자열 가공으로 교체 필요.
원하시면 값 객체 + Converter 방식으로 안전하게 리팩터링하는 PR 패치를 드리겠습니다.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Enumerated(EnumType.STRING) | |
| @Column(name = "admission_semester", nullable = false, length = 10) | |
| private AdmissionSemester admissionSemester; | |
| // 과거/미래 학기 값을 폭넓게 수용하기 위해 문자열로 저장합니다. | |
| @Column(name = "admission_semester", nullable = false, length = 10) | |
| private String admissionSemester; |
🤖 Prompt for AI Agents
In src/main/java/inha/gdgoc/domain/recruit/entity/RecruitMember.java around
lines 77-79, the AdmissionSemester field is mapped as an Enum with
@Enumerated(EnumType.STRING) which will fail loading legacy DB values like
"Y24_1" causing IllegalArgumentException; change the field to a String (persist
raw code), add validation (DB CHECK + Bean Validation pattern) and introduce
either a domain value object or an AttributeConverter to parse/format semester
codes for domain logic; then update callers (e.g.,
RecruitMemberRequest.toEntity() to supply the semester code string instead of an
enum, and DTO mappings that used enum.name() to use the string) — Option A
(store String + value object/validator) is recommended, Option B
(AttributeConverter) acceptable, Option C (expanding enum) is last-resort.
| public enum AdmissionSemester { | ||
| Y25_1, Y25_2, Y26_1, Y26_2 | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
열거값 범위 한정으로 인한 런타임 예외 위험: 1월 처리와 향후 연도 대비 필요
SemesterCalculator.of()는 1월을 “이전 해 2학기”로 산출합니다. 예: 2025-01 → Y24_2. 현재 enum에는 Y24_2가 없어 해당 시점 호출 시 예외가 발생합니다. 또한 2028-01(→ Y27_2)도 대비가 없습니다. 최소 과거 1개 학기 및 향후 2~3년치를 선반영하세요.
권장 수정안(예시: 과거 24년 2학기, 미래 27년까지 확장):
public enum AdmissionSemester {
- Y25_1, Y25_2, Y26_1, Y26_2
+ Y24_2, Y25_1, Y25_2, Y26_1, Y26_2, Y27_1, Y27_2
}추가로, 매년 연말 CI에서 다음 해 상수 누락을 검출하는 가드 테스트를 두는 것을 권장합니다.
다음 스크립트로 AdmissionSemester 열거값과 SemesterCalculator.of() 구현을 검증해 보겠습니다.
🏁 Script executed:
#!/bin/bash
# AdmissionSemester enum 확인
echo '== AdmissionSemester enum =='
sed -n '1,200p' src/main/java/inha/gdgoc/domain/recruit/enums/AdmissionSemester.java
# SemesterCalculator 클래스 위치 확인
echo '== SemesterCalculator 클래스 위치 =='
rg -n 'class SemesterCalculator' -C3 src/main/java/inha/gdgoc/domain/recruit
# SemesterCalculator.of() 구현 확인
echo '== SemesterCalculator.of() 구현 =='
rg -nP 'public static.*AdmissionSemester of' -C5 src/main/java/inha/gdgoc/domain/recruitLength of output: 576
SemesterCalculator.of() 구현을 직접 확인하기 위해 전체 프로젝트에서 해당 클래스와 메서드를 찾아보겠습니다.
🏁 Script executed:
#!/bin/bash
# 1) SemesterCalculator 클래스 파일 경로 조회
FILE=$(rg -l 'class SemesterCalculator' -g '*.java' .)
echo "SemesterCalculator 위치: $FILE"
# 2) 클래스 상단 200줄 출력
echo "== SemesterCalculator 초기 부분 =="
sed -n '1,200p' "$FILE"
# 3) of() 메서드 구현부 출력
echo "== of() 메서드 구현 =="
rg -n 'of\s*\(' -C5 "$FILE"
# 4) 연도·월 계산 로직 확인 (getYear, getMonthValue 탐색)
echo "== 연도·월 계산 참조 =="
rg -n -C3 'getYear|getMonthValue' "$FILE"Length of output: 2626
AdmissionSemester enum에 과거·미래 학기 추가 필요
SemesterCalculator.of()에서 1월(month == 1)에 이전 해 2학기(예: 2025-01 →Y24_2)를 참조하도록 설계되어 있으나, 현재AdmissionSemester에Y24_2가 없어 2025년 1월 호출 시 런타임 예외 발생- 향후 2년치(2027년) 학기(2학기:
Y27_2, 1학기:Y27_1)도 누락되어 Feb–Jul 또는 Aug–Jan(1월) 호출 시 예외 위험
대응 방안(예시):
--- a/src/main/java/inha/gdgoc/domain/recruit/enums/AdmissionSemester.java
+++ b/src/main/java/inha/gdgoc/domain/recruit/enums/AdmissionSemester.java
@@ public enum AdmissionSemester {
- Y25_1, Y25_2, Y26_1, Y26_2
+ Y24_2, Y25_1, Y25_2, Y26_1, Y26_2, Y27_1, Y27_2
}- 매년 연말 또는 CI 파이프라인에서 다음 해·학기 상수 누락을 감지하는 가드 테스트 추가를 권장합니다.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public enum AdmissionSemester { | |
| Y25_1, Y25_2, Y26_1, Y26_2 | |
| } | |
| public enum AdmissionSemester { | |
| Y24_2, Y25_1, Y25_2, Y26_1, Y26_2, Y27_1, Y27_2 | |
| } |
🤖 Prompt for AI Agents
In src/main/java/inha/gdgoc/domain/recruit/enums/AdmissionSemester.java around
lines 3 to 5, the enum lacks past and future semester constants (e.g., Y24_2 and
upcoming Y27_1/Y27_2), causing SemesterCalculator.of() to throw at runtime for
months that reference previous or future years; add the missing semester enum
values covering at least the previous year’s second semester (Y24_2) and the
next two years’ semesters (e.g., Y26_1/Y26_2 already present so add Y27_1 and
Y27_2) so all month-to-semester mappings resolve, and consider adding a CI/unit
test that verifies the enum contains the expected range of year/semester
constants each release.
📌 연관된 이슈
✨ 작업 내용
💬 리뷰 요구사항(선택)
Summary by CodeRabbit
New Features
Chores